Docs: mockClear, mockReset, mockRestore
#6227
Conversation
| this._mockConfigRegistry.delete(f); | ||
| return f; | ||
| }; | ||
|
|
||
| f.mockRestore = () => { | ||
| f.mockReset(); | ||
| return restore ? restore() : undefined; |
There was a problem hiding this comment.
Ideally I would like this to be return f, to be consistent with the other methods - but keeping the API exactly the same, this is the current behaviour.
stephencookdev
force-pushed
the
feature/mock-documentation
branch
from
7806b26 to
2dc4291
Compare
|
CI is failing, mind taking a look?
Travis is failing (we really should remove it), but circle and appveyor is fine: |
|
Ah, I didn't check the state of the UTs closely enough, then 😞 Thanks for the info So it looks like the failure is the following:
Which I didn't actually realise. So it means I misunderstood (again 😅) the behaviour of This means that But is there any use-case for wanting But I can either fix this by
|
|
@rickhanlonii ^ ideas? |
stephencookdev
changed the title
mockClear, mockReset, mockRestore
docs/MockFunctionAPI.md
Outdated
| Resets all information stored in the mock, including any initial implementation | ||
| and mock name given. | ||
| Does everything that [`mockFn.mockClear()`](#mockfnmockclear) does, and also | ||
| removes any mocked return values. |
There was a problem hiding this comment.
Any mocked return values or implementations
docs/MockFunctionAPI.md
Outdated
| state. | ||
| This is useful when you want to completely restore a _mock_ back to its initial | ||
| state. (Note that resetting a _spy_ will result in a function with no return | ||
| value). |
There was a problem hiding this comment.
I wouldn't use the word "restore" here, what do you think about using "completely reset a mock"
There was a problem hiding this comment.
Yes, I agree :) Will change
docs/MockFunctionAPI.md
Outdated
| @@ -116,7 +117,8 @@ don't access stale data. | |||
|
|
|||
| ### `mockFn.mockRestore()` | |||
|
|
|||
| Removes the mock and restores the initial implementation. | |||
| Does everything that [`mockFn.mockReset()`](#mockfnmockreset) does, and also | |||
| restores the original (non-mocked) implementation. | |||
There was a problem hiding this comment.
Should this say:
restores the original (non-mocked) implementation for spies
There was a problem hiding this comment.
Yeah, that probably makes more sense 👍
There was a problem hiding this comment.
I think we probably just forgot about removing the mockName for spies when we added the mock names, so it's good to remove (though it may be a breaking bugfix)
stephencookdev
force-pushed
the
feature/mock-documentation
branch
from
2dc4291 to
5cc2a0d
Compare
|
Thanks for the review @rickhanlonii - addressed your comments, and made it so that |
stephencookdev
force-pushed
the
feature/mock-documentation
branch
from
5cc2a0d to
e934dce
Compare
Codecov Report
@@ Coverage Diff @@
## master #6227 +/- ##
=========================================
+ Coverage 63.89% 63.9% +<.01%
=========================================
Files 228 228
Lines 8705 8707 +2
Branches 4 4
=========================================
+ Hits 5562 5564 +2
Misses 3142 3142
Partials 1 1
Continue to review full report at Codecov.
|
| @@ -121,6 +121,7 @@ | |||
|
|
|||
| ### Chore & Maintenance | |||
|
|
|||
| * `[docs]` Improve documentation of `mockClear`, `mockReset`, and `mockRestore` ([#6227](https://github.com/facebook/jest/pull/6227/files)) | |||
There was a problem hiding this comment.
Needs moved to the master section
There was a problem hiding this comment.
I think this got messed up after a merge conflict - cheers 😓 Moved back to master :)
| @@ -0,0 +1,310 @@ | |||
| --- | |||
There was a problem hiding this comment.
Can only add this to the master docs? Will ship in the 23.1 version
There was a problem hiding this comment.
Ah, I misunderstood the process. Yeah, np - will remove my versioned_docs changes
…re` (especially surrounding `spyOn` behaviour)
stephencookdev
force-pushed
the
feature/mock-documentation
branch
from
e934dce to
10f1c6d
Compare
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
This PR is aiming to relieve some of the difficulties flagged up in #4828 and #5143
Specifically, the exact behaviour of
mockClear,mockReset, andmockRestoreisn't massively clear. I personally felt this pain when trying to debug an issue that turned out to bemockResetbeing called on a spy, "resetting" it to an empty mock (rather than my original assumption, of the original implementation at the time the spy was attached).I've also made a minor change to the actual source code of
mockClearetc., but only to make it more explicit in the code thatmockClear<mockReset<mockRestore(which was already the case, it's just now more obvious at a glance).Test plan
The website changes appear as expected by
cd website ; yarn start.yarn testyields no (or at least, no additional, since there are errors on master) failures